-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Document MCF as another volume/gas/water unit #2746
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughDocumentation updates: added the unit "MCF" (thousand cubic feet) to GAS, VOLUME, VOLUME_STORAGE, and WATER device classes; additionally expanded the list of available sensor device classes in Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/core/entity/number.md (1)
53-53
: MCF addition looks correct; consider clarifying the abbreviation once.The unit list now contains
MCF
, which is great. BecauseM
can ambiguously stand for thousand or million in different domains, a one-time footnote or short parenthetical “(thousand ft³)” elsewhere in the doc would pre-empt confusion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (2)
docs/core/entity/number.md
(2 hunks)docs/core/entity/sensor.md
(2 hunks)
🔇 Additional comments (7)
docs/core/entity/number.md (3)
82-82
: LGTM – unit list kept in sync with GAS row.
84-84
: LGTM – VOLUME_STORAGE now lists MCF.
85-85
: LGTM – WATER row updated consistently.docs/core/entity/sensor.md (4)
56-56
: MCF unit added for GAS sensors – looks good.
86-86
: Volume sensor units updated – in sync with number docs.
88-88
: MCF inclusion for VOLUME_STORAGE confirmed.
89-89
: Water sensor row updated – no issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please resolve the conflicts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @pioto 👍
../Frenck
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved the conflicts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
docs/core/entity/number.md (2)
53-53
: Clarify that MCF = thousand cubic feetAdd a short clarification to avoid ambiguity with “million cubic feet” used elsewhere.
-| `NumberDeviceClass.GAS` | L, m³, ft³, CCF, MCF | Volume of gas. Gas consumption measured as energy in kWh instead of a volume should be classified as energy. +| `NumberDeviceClass.GAS` | L, m³, ft³, CCF, MCF | Volume of gas. Gas consumption measured as energy in kWh instead of a volume should be classified as energy. “MCF” denotes thousand cubic feet.
82-82
: Grammar: “represent consumption”Tighten phrasing.
-| `NumberDeviceClass.VOLUME` | L, mL, gal, fl. oz., m³, ft³, CCF, MCF | Generic volume, this device class should be used to represent a consumption, for example the amount of fuel consumed by a vehicle. +| `NumberDeviceClass.VOLUME` | L, mL, gal, fl. oz., m³, ft³, CCF, MCF | Generic volume, this device class should be used to represent consumption, for example the amount of fuel consumed by a vehicle.docs/core/entity/sensor.md (2)
56-56
: Clarify that MCF = thousand cubic feetMirror the clarification here as well for consistency with number.md.
-| `SensorDeviceClass.GAS` | L, m³, ft³, CCF, MCF | Volume of gas. Gas consumption measured as energy in kWh instead of a volume should be classified as energy. +| `SensorDeviceClass.GAS` | L, m³, ft³, CCF, MCF | Volume of gas. Gas consumption measured as energy in kWh instead of a volume should be classified as energy. “MCF” denotes thousand cubic feet.
86-86
: Grammar: “representing consumption”Minor style tweak.
-| `SensorDeviceClass.VOLUME` | L, mL, gal, fl. oz., m³, ft³, CCF, MCF | Generic volume, this device class should be used for sensors representing a consumption, for example the amount of fuel consumed by a vehicle. +| `SensorDeviceClass.VOLUME` | L, mL, gal, fl. oz., m³, ft³, CCF, MCF | Generic volume, this device class should be used for sensors representing consumption, for example the amount of fuel consumed by a vehicle.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
docs/core/entity/number.md
(2 hunks)docs/core/entity/sensor.md
(2 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/core/entity/sensor.md
[grammar] ~86-~86: There might be a mistake here.
Context: ...he amount of fuel consumed by a vehicle. | SensorDeviceClass.VOLUME_FLOW_RATE
|...
(QB_NEW_EN)
[grammar] ~87-~87: There might be a mistake here.
Context: ...he amount of water consumed momentarily. | SensorDeviceClass.VOLUME_STORAGE
| L...
(QB_NEW_EN)
[grammar] ~88-~88: There might be a mistake here.
Context: ...ample the amount of fuel in a fuel tank. | SensorDeviceClass.WATER
| L, gal, m³...
(QB_NEW_EN)
[grammar] ~89-~89: There might be a mistake here.
Context: ...l, m³, ft³, CCF, MCF | Water consumption | SensorDeviceClass.WEIGHT
| kg, g, mg...
(QB_NEW_EN)
docs/core/entity/number.md
[grammar] ~82-~82: There might be a mistake here.
Context: ...he amount of fuel consumed by a vehicle. | NumberDeviceClass.VOLUME_FLOW_RATE
|...
(QB_NEW_EN)
[grammar] ~83-~83: There might be a mistake here.
Context: ...he amount of water consumed momentarily. | NumberDeviceClass.VOLUME_STORAGE
| L...
(QB_NEW_EN)
[grammar] ~84-~84: There might be a mistake here.
Context: ...ample the amount of fuel in a fuel tank. | NumberDeviceClass.WATER
| L, gal, m³...
(QB_NEW_EN)
[grammar] ~85-~85: There might be a mistake here.
Context: ...l, m³, ft³, CCF, MCF | Water consumption | NumberDeviceClass.WEIGHT
| kg, g, mg...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Redirect rules - developers-home-assistant
- GitHub Check: Header rules - developers-home-assistant
- GitHub Check: Pages changed - developers-home-assistant
🔇 Additional comments (10)
docs/core/entity/number.md (5)
53-53
: Add MCF to GAS — LGTMConsistent with other docs and unit style.
82-82
: Add MCF to VOLUME — LGTM
84-84
: Add MCF to VOLUME_STORAGE — LGTM
85-85
: Add MCF to WATER — LGTM
53-85
: Avoid doc drift: confirm core + conversion support for MCFEnsure the linked core PR (#150015) adds MCF to unit constants, conversion/normalization, and any unit system mappings so statistics and UI behave correctly.
docs/core/entity/sensor.md (5)
56-56
: Add MCF to GAS — LGTM
86-86
: Add MCF to VOLUME — LGTM
88-88
: Add MCF to VOLUME_STORAGE — LGTM
89-89
: Add MCF to WATER — LGTM
86-89
: Check statistics normalization for MCFPlease confirm Recorder/Statistics normalization includes MCF for GAS/VOLUME/WATER to allow mixed-unit charts and consistent long-term stats.
Oh, thanks for fixing the merge conflict while I slept! :) |
Proposed change
Complementary to home-assistant/home-assistant.io#40281 and home-assistant/core#150015, this documents MCF as another alternative volume unit, as this is how some utilities in the US bill for water or gas.
Type of change
Checklist
Additional information
Summary by CodeRabbit